Make NativeAOT StressLog binary-compatible with CoreCLR#129300
Draft
max-charlamb wants to merge 1 commit into
Draft
Make NativeAOT StressLog binary-compatible with CoreCLR#129300max-charlamb wants to merge 1 commit into
max-charlamb wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns the NativeAOT StressLog header layout and logging entrypoints toward CoreCLR compatibility so SOS / clrmd tooling can interpret stress log memory without runtime-specific parsing.
Changes:
- Replaces the inline mutex with a pointer-sized heap-allocated
CrstStatic*and updates locking in thread-log creation. - Adds CoreCLR-matching header fields (
paddingsentinel,ModuleDesc modules[5]) and initializes the module table. - Updates
StressLog::LogMsg/STRESS_LOG_*plumbing to carry(level, facility, ...)in the signature.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/nativeaot/Runtime/stressLog.cpp | Switches StressLog lock implementation to CrstStatic*, initializes module table, updates LogMsg signature. |
| src/coreclr/nativeaot/Runtime/inc/stressLog.h | Updates macros and LogMsg signature; adds padding + modules[] fields and static_assert-based layout checks. |
Comments suppressed due to low confidence (1)
src/coreclr/nativeaot/Runtime/stressLog.cpp:378
- StressLog::LogMsg starts a va_list but returns early (pThread == NULL / msgs == 0) without calling va_end, and the new
levelparameter is unused. This is undefined behavior and can trigger -Werror unused-parameter builds.
Consider ensuring va_end is always reached and explicitly discarding level if it is intentionally not stored in the StressMsg payload.
void StressLog::LogMsg (unsigned level, unsigned facility, int cArgs, const char* format, ... )
{
_ASSERTE ( cArgs >= 0 && (size_t)cArgs <= StressMsg::maxArgCnt );
va_list Args;
Contributor
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
max-charlamb
force-pushed
the
maxcharlamb/nativeaot-stresslog-unify
branch
from
14b1ddd to
5da9202
Compare
Align the NativeAOT StressLog struct layout to match CoreCLR's, so that diagnostic tools reading raw memory (SOS !DumpLog, clrmd) can read stress logs from both runtimes without layout-specific code paths. Changes: - Add padding field (0xFFFFFFFF sentinel) between logs and deadCount - Change lock from inline minipal_mutex to heap-allocated CrstStatic* (pointer-sized, matching CoreCLR's CRITSEC_COOKIE) - Add modules[MAX_MODULES=5] array with ModuleDesc struct - Update StressLog::LogMsg signature to (level, facility, cArgs, fmt) matching CoreCLR's convention - Update STRESS_LOG_WRITE macro and LogMsgOL overloads to pass level - Add static_asserts verifying field offsets match CoreCLR layout - Add null guard for lock in CreateThreadStressLog The lock change uses CrstStatic which wraps the same minipal_mutex primitives, so runtime behavior is identical. In Debug builds, CrstStatic adds thread ownership assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
max-charlamb
force-pushed
the
maxcharlamb/nativeaot-stresslog-unify
branch
from
5da9202 to
e2d749d
Compare
Copilot stopped reviewing on behalf of
max-charlamb due to an error
June 11, 2026 19:08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This PR was created with assistance from GitHub Copilot.
Summary
Align the NativeAOT
StressLogstruct layout to be binary-compatible with CoreCLR's, so that diagnostic tools reading raw memory (SOS!DumpLog, clrmd) can read stress logs from both runtimes without layout-specific code paths.Motivation
The NativeAOT and CoreCLR
StressLogstructs were forked from the same file but diverged over time. WhileStressMsg,StressLogChunk, andThreadStressLogremained compatible, theStressLogheader class itself has different field ordering due to:CRITSEC_COOKIE(an 8-byte pointer). NativeAOT usedminipal_mutex(40 bytes inline on Linux x64).paddingsentinel: CoreCLR hasunsigned padding = 0xFFFFFFFFbetweenlogsanddeadCount.modules[]array: CoreCLR has a 5-entryModuleDescarray for multi-module format string resolution.LogMsgsignature: CoreCLR passes(level, facility, ...), NativeAOT only passed(facility, ...).These differences meant every field after
logswas at a different offset, breaking raw-memory readers.Changes
Lock type (
stressLog.h,stressLog.cpp)minipal_mutexto heap-allocatedCrstStatic*CrstStaticwraps the sameminipal_mutexprimitives, so runtime behavior is identicalCrstStaticadds thread ownership assertions (strictly better)CreateThreadStressLogfor the unlikely allocation failure caseStruct layout (
stressLog.h)unsigned paddingfield (initialized to0xFFFFFFFF) betweenlogsanddeadCountModuleDescstruct andmodules[MAX_MODULES=5]arraystatic_asserts that verify all field offsets match the expected CoreCLR layoutAPI signatures (
stressLog.h,stressLog.cpp)StressLog::LogMsgto(unsigned level, unsigned facility, int cArgs, const char* format, ...)STRESS_LOG_WRITEmacro to passlevelthrough toLogMsgOLLogMsgOL2-arg overload to(unsigned facility, unsigned level, const char* format, Ts... args)STRESS_LOG_PLUG_MOVE,STRESS_LOG_ROOT_PROMOTE, etc.) to pass the appropriate levelRelated PRs
ISOSDacInterface17for cDAC-based stress log reading!DumpLogupdated to useISOSDacInterface17